Repository navigation
perf: skip collected string handles during GC marking - #30
Conversation
284km
left a comment
284km
left a comment
There was a problem hiding this comment.
Reviewed dd37a25 against base 8977532. No actionable findings.
The sweep preserves the sorted order of the surviving prefix, then appends newly allocated slots in increasing order. The new searches respect that prefix's logical length and the previous allocation boundary. This preserves live empty strings while excluding reclaimed handles, without introducing another liveness table or changing GC thresholds.
Validation on macOS with fresh O2 builds of both revisions, using the same pinned Mere compiler 01ed9680 and pinned mgz dependency:
run_corpus.sh --both: 504/504 programs matched Ruby 4.0.9 with the hash index ON and OFF; the preceding source/record checks also passed.- The new regression and five existing GC regressions matched Ruby in both index modes with
MERE_RUBY_GC_STRESS=100. These cover collections inside methods and bodies, suspended Fiber values, and hash retention. - The standalone benchmark printed the same result in both builds. Across three runs per revision, the median timed collection section changed from 0.366 s to 0.0225 s; peak mark-table entries changed from 301,768 to 1,772.
git diff --checkpassed.
Validation limits: the full new regression with stress period 1 exceeded the 180-second local deadline and remains inconclusive at that setting. I did not independently rerun the selected upstream Unicode timing or Linux execution. The broader CI measurement shards are still running.
| if slot < 0 || slot >= str_count () then false | ||
| else if slot >= vec_get gc_knob 8 then true | ||
| else (let j = gc_str_surv_lower slot in | ||
| j < vec_get gc_knob 10 && vec_get gc_str_surv j == slot); |
There was a problem hiding this comment.
Good distinction between logical and physical length: gc_str_surv can retain stale entries after its live prefix has been compacted. Searching only the prefix counted by gc_knob[10], while treating allocations since gc_knob[8] separately, avoids reviving those stale entries and keeps genuinely live empty strings eligible for marking.
There was a problem hiding this comment.
Thanks for checking the sweep invariant. The search remains bounded by the logical survivor prefix, and newly allocated slots are handled from the previous allocation boundary. This keeps stale physical entries out of the mark set while preserving live empty strings. No code change is needed here.
| 12000.times do |i| | ||
| value = "#{round}:#{i}" | ||
| kept << [value, value.dup, ''.encode(Encoding::UTF_16LE)] if i % 6000 == 0 | ||
| end |
There was a problem hiding this comment.
The empty encoded strings are useful coverage here: a live empty string and a reclaimed slot can have identical contents, so content-based liveness would silently lose encoding or identity. This regression matched Ruby in both index modes, including stress period 100.
There was a problem hiding this comment.
Thanks for independently checking this in both index modes with stress period 100. I have retained the empty encoded strings and the alias/capture checks. The PR description now records your additional validation and explicitly leaves the full regression at stress period 1 inconclusive after the 180-second deadline; it is not counted as a pass. All 29 hosted CI jobs have also completed successfully.
A half-evaluated expression can retain a conservative allocation interval spanning strings that an earlier collection already reclaimed. The collector previously treated every issued string ID in that interval as present, repeatedly inserting dead IDs into the mark table and scanning recycled pages.
Use the existing, sorted survivor list from the previous sweep to identify old strings that still exist. Interval marking walks those survivors plus newly allocated slots, and conservative stack candidates use the same membership check. Live empty strings remain distinguishable from collected strings; no additional liveness table or GC threshold change is needed.
Local measurements at O2 with the same pinned compiler (
01ed9680):TestComprehensiveCaseMapping#test_UTF-8_upcase_asciichanges from 136.62 s to 60.01 s, with 40,569 assertions, zero failures and zero errors in both runs. This measures one selected test, not completion of the entire Unicode file or a whole-workflow speedup.Validation:
dd37a25passed all 29 jobs. The report collected all 758 planned CRuby test files from 8 shards with no missing files or infrastructure errors. Successful collection does not mean full Ruby conformance.git diff --checkpasses.Based on main at
8977532; this GC change is independent of #29 and the now-merged Mere UDP runtime PR.Independent review of
dd37a25found no actionable issues and reproduced the corpus results and benchmark improvement. The new regression and five existing GC regressions also matched Ruby in both index modes withMERE_RUBY_GC_STRESS=100. The full new regression at stress period 1 exceeded the reviewer's 180-second deadline, so that setting remains inconclusive. The complete upstream Unicode case-mapping file still reaches its file-level wall limit in CI; the selected-test timing above does not claim otherwise.